fix(antigravity): use temp file for prompt instead of stdin (fixes #5… - #6332
fix(antigravity): use temp file for prompt instead of stdin (fixes #5…#6332AadiyKhan wants to merge 8 commits into
Conversation
|
Hey @AadiyKhan — thanks for the focused Antigravity adapter fix here. This looks aimed at the |
mrcfps
left a comment
There was a problem hiding this comment.
@AadiyKhan, thank you for tackling the Antigravity prompt-delivery regression and for validating the direction on Windows. I found two concrete blockers in the changed runtime path: overlapping runs can replace one another's prompt file, and the existing focused daemon test now fails. The inline comments describe a small fix using the prompt-file lifecycle infrastructure already present in the daemon.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| const tempFile = join(tmpdir(), `od_agy_prompt_${process.pid}.md`); | ||
| writeFileSync(tempFile, _prompt); |
There was a problem hiding this comment.
Blocking — give each run a managed prompt file. This filename is derived only from the daemon PID, so every Antigravity run in the process writes the same path. The daemon explicitly allows overlapping runs and invokes buildArgs before each child reads the file; calling this implementation twice already produces the same prompt argument and leaves only the second transcript on disk. In production, run A can therefore execute run B's system instructions, history, and user request. The file is also never removed and is written with process-default permissions, leaving the latest full transcript in the shared OS temp directory after the run. Please opt this definition into promptViaFile: true, require runtimeContext.promptFilePath here, and let the existing preparePromptFileForAgent path create a unique mode-0600 file per run and clean it after child exit. Add coverage for two prepared runs retaining distinct contents and for cleanup.
| return args; | ||
| }, | ||
| promptViaStdin: true, | ||
| promptViaStdin: false, |
There was a problem hiding this comment.
Blocking — update the focused regression test with the transport contract. This line changes promptViaStdin to false, but apps/daemon/tests/runtimes/agent-args.test.ts still asserts that it is true and that every argument list ends in ['-p', '-']. On this head, vitest run -c vitest.config.ts tests/runtimes/agent-args.test.ts fails at line 540 (false !== true), so the daemon test lane is red and none of the new file behavior is pinned. Please update that Antigravity case to assert the intended file transport and argument shape, including the log-file and follow-up variants; if the existing managed prompt-file path is used, also assert promptViaFile: true and that a missing promptFilePath fails clearly.
|
Hey @lefarcen! Thanks for linking the other PRs. I wanted to quickly clarify why I went with the temp-file architecture here, as it directly solves the
The Solution: Let me know if you need any adjustments to the code! |
|
Thanks for the clarification, @AadiyKhan — the Windows At this point the blocking items to address are the two from @mrcfps on the current head: switching this over to the daemon's managed prompt-file lifecycle so concurrent runs don't share one temp path, and updating the focused runtime test to match the new transport contract. Once those are fixed on this PR, the reviewers can re-check this approach against the other Antigravity candidates. 💡 To drive this PR to merge hands-free, paste this to your AI coding agent (Claude Code / Codex / opencode / Cursor …): |
|
Hey @lefarcen — you caught me right in the middle of pushing the fix for those! I've just pushed a new commit that addresses both of the blockers raised by @mrcfps:
All local tests are green on my end. Let me know if the reviewers need anything else to get this over the finish line! |
mrcfps
left a comment
There was a problem hiding this comment.
Thank you for the focused follow-up on the Antigravity prompt-delivery fix. I verified that the latest head replaces the shared PID path with the daemon's per-run managed prompt-file lifecycle, preserves the print-mode and log-file argument contract, and updates the runtime regression coverage for chat, logging, model selection, and follow-up turns. The focused runtime tests pass (47/47), and the daemon typecheck plus repository guard are green. Nice work addressing both earlier blockers cleanly.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.|
@lefarcen @mrcfps I also wanted to share that this is my very first open-source contribution! It was super exciting to track down this Windows bug, and I really appreciate how welcoming and structured the review process was. Thanks again for the help getting this over the finish line! Let me know when it's merged! |
|
Nice turnaround on the follow-up, @AadiyKhan — and congrats on your first open-source contribution. This PR is now queued for QA validation because it changes a live runtime path that needs one manual pass before merge. We’ll update the thread again once that validation is done. |
|
While waiting for QA validation, I realized there was a massive opportunity to make the antigravity adapter far more powerful than the sandboxed agents, so I just pushed one more commit with two major structural upgrades:
The unit tests (agent-args.test.ts) have been fully updated to assert the new workspace arguments and the augmented system prompt. This commit still perfectly preserves the streamFormat: 'plain' contract, so it shouldn't affect the pending QA validation of the core temp-file fix. Let me know what you think! |
mrcfps
left a comment
There was a problem hiding this comment.
@AadiyKhan, thank you for the thoughtful managed prompt-file follow-up and for continuing to improve the Antigravity integration. The transport fix, focused runtime test, daemon typecheck, and repository guard all validate cleanly locally. The latest commit does introduce one blocking safety/correctness issue: it tells the model that host-wide authorization exists even though Antigravity still enforces the user's separate permission and sandbox policy. I’ve left one focused inline comment with the concrete correction.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.|
Thanks for flagging this, @AadiyKhan. This latest push broadens the PR beyond the prompt-file fix that was previously approved: binding |
|
Thanks for the quick follow-up here, @AadiyKhan. The current blocker is now the one @mrcfps called out on this head: the new host-access override needs to come back out unless and until it’s backed by a real user-controlled Antigravity permission setting. Once that is reverted and the head is updated, reviewers can re-check the narrower transport/workspace change on its own. |
|
Good catch, @mrcfps! That's a great point about the conflict between the prompt assertion and the actual CLI sandbox policy. We definitely don't want the model hallucinating permissions it doesn't really have.I have pushed a follow-up commit that completely reverts the system override text back to the narrow, managed-file instruction we agreed on. However, I have left the --add-dir workspace binding in place, as that safely guarantees the CLI drops into the correct project folder without needing extra authorization. |
|
Thanks for turning that around so quickly, @AadiyKhan. From the current thread state, the specific blocker on the last head looks addressed: @mrcfps already resolved the review thread against |
mrcfps
left a comment
There was a problem hiding this comment.
@AadiyKhan, thank you for the careful follow-up on the Antigravity prompt transport. The managed per-run file lifecycle is sound, the unsafe host-authorization wording is gone, and the focused tests, daemon typecheck, and repository guard all pass locally. I found one permission-policy compatibility gap in which directory is added to the Antigravity workspace; it is merge-safe but worth tightening so restricted and sandboxed configurations can consume the managed prompt reliably.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.|
Thanks for tightening this up, @AadiyKhan. On the current head, the thread has moved from blocking to non-blocking: @mrcfps’s latest note is about tightening the workspace directory passed to Antigravity so sandboxed/restricted configurations can reliably read the managed prompt file, but it isn’t holding the PR. So from here the main thing still pending is QA finishing its validation pass on this head; the directory tweak can be folded in if you want to address the compatibility edge case before merge. |
|
Good call, @mrcfps. Since the spawn process already implicitly mounts the project folder via its cwd, passing it explicitly via --add-dir was redundant.However, adding the temp directory to the workspace is a brilliant safeguard to ensure that restrictive sandbox policies don't accidentally block the agent from reading the prompt file itself.I have pushed a new commit that replaces the redundant cwd logic with dirname(runtimeContext.promptFilePath). I also updated the deepEqual assertions in the runtime test to expect the prompt directory instead. |
mrcfps
left a comment
There was a problem hiding this comment.
@AadiyKhan, thank you for following through on the Antigravity managed prompt-file fix and for correcting the workspace directory on the latest head. The runtime path now uses the daemon’s unique, cleaned-up prompt file and grants Antigravity access to its containing directory. One focused regression-test inconsistency remains: the new unconditional --add-dir argument is reflected in the first two expectations but omitted from three later cases, so the changed test cannot pass as written. I’ve consolidated the exact correction into one inline comment.
|
Thanks for tightening that up, @AadiyKhan. On the current head, the remaining blocker is the focused test mismatch @mrcfps called out: |
|
Thanks for catching that, @mrcfps! Complete oversight on my part—since --add-dir is applied unconditionally for the prompt file, it naturally alters the argv shape for the model-selection and follow-up turn test cases as well. I've pushed a commit that adds the --add-dir, /tmp pair to the remaining three deepEqual assertions. I ran the suite locally and it passes cleanly now! Let me know if everything looks good to go. |
mrcfps
left a comment
There was a problem hiding this comment.
Thank you for the careful follow-through on this Antigravity prompt-delivery fix. I verified the final composed change now routes the transcript through the daemon's unique managed prompt file, grants agy access to that file's directory while preserving the project working directory, and keeps the existing logging and model-selection argument order intact. The focused runtime argument tests (45/45), prompt-file lifecycle tests (2/2), daemon typecheck, and repository guard all pass locally. Nice work addressing the earlier concurrency, permission, and test-consistency concerns cleanly. 🙌
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.|
Thanks for the quick cleanup here, @AadiyKhan. The three stale expectations are now aligned with the unconditional |
|
Hey @lefarcen @mrcfps,
|
mrcfps
left a comment
There was a problem hiding this comment.
@AadiyKhan, thank you for the careful follow-through on the Antigravity prompt-delivery fix. I verified that the managed per-run prompt transport remains intact and that the focused runtime tests (45/45), daemon typecheck, repository guard, and live checks pass. The latest atomic-write follow-up introduces one merge-safe POSIX permission regression; I’ve left a focused inline suggestion to preserve the settings file’s existing protection. Nice work continuing to tighten this integration. 🙌
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.|
Thanks for the extra hardening pass here, @AadiyKhan. The one thing still worth tightening on this head is @mrcfps's note about preserving the existing permissions on |
|
@mrcfps Spot on. Overwriting a strict 0600 policy file with a default 0644 mask during the atomic swap would have been a nasty permission regression.I have pushed a follow-up commit that calls statSync to capture the existing destination's mode (defaulting to 0o600 if the file is fresh) and explicitly passes it to writeFileSync. I also added the focused test case to agent-args.test.ts to assert that the mode is perfectly preserved across the atomic replacement boundary. |
mrcfps
left a comment
There was a problem hiding this comment.
@AadiyKhan, thank you for the careful Antigravity transport and settings-file hardening. The managed prompt-file path, permission preservation, focused argument test, daemon typecheck, and repository guard all validate cleanly. One log-argument compatibility regression remains in the broader connection-test seam; because this reviewer has reached the PR's request-changes cap, I am leaving it as a comment for maintainer disposition.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.| // and auth/quota failures. | ||
| if (runtimeContext.agentLogFilePath) { | ||
| args.push('--log-file', runtimeContext.agentLogFilePath); | ||
| args.push(`--log-file=${runtimeContext.agentLogFilePath}`); |
There was a problem hiding this comment.
Maintainer attention — keep the silent-failure log contract covered. Changing this option to the single-token --log-file=… form leaves the existing fake Antigravity CLI in apps/daemon/tests/connection-test.test.ts unable to find the requested log path: that fixture looks up the exact --log-file token and reads the following argument. On this head, pnpm exec vitest run -c vitest.config.ts tests/connection-test.test.ts fails 1 of 156 tests; the quota-exhaustion case receives agent_auth_required instead of the expected rate_limited because the diagnostic log is never written. This matters because that test pins the user-visible distinction between OAuth recovery and quota recovery, and the repository approval bar requires the matching package tests to pass. Please either retain the previously validated two-token form here, or update the fake CLI to accept the equals form after validating that the supported agy versions do too, then rerun the connection-test file.
|
Thanks for the follow-up here, @AadiyKhan. The remaining item on this head is the broader connection-test seam @mrcfps called out above: the fake Antigravity CLI still parses |
Why
• Your use case: I hit this myself on Windows while trying to configure Open Design to use the Antigravity CLI (agentId: "antigravity"). The
integration would just hang and return the default agy terminal greeting.
• The pain being addressed: The antigravity adapter previously used -p - to pass the context via stdin. However, as tracked in #5495, recent versions
of the Antigravity CLI (agy) treat - as a literal prompt string instead of reading from stdin. This caused agy to ignore the Open Design CRITIQUE_RUN
protocol entirely. This PR safely routes the prompt through a temporary file instead.
What users will see
• The Antigravity CLI integration now correctly executes the Open Design orchestrator workflow instead of instantly returning the default terminal
greeting.
Surface area
[ ] UI
[ ] Keyboard shortcut
[ ] CLI / env var
[ ] API / contract
[ ] Extension point
[ ] i18n keys
[ ] New top-level dependency
[ ] Default behavior change
[✓] None — internal refactor, docs, tests, or translation update only
Screenshots
(N/A - backend adapter fix only)
Bug fix verification
• Test path that reproduces the bug: Verified manually on Windows client.
• Did the test go red on main and green on this branch?: no
• If a red spec wasn't cheap to write, explain why and what verification you did instead: The interaction with the external agy CLI binary's stdin
behavior makes this hard to unit test without mocking the binary execution itself. Verified manually by confirming od_agy_prompt_${process.pid}.md is
created correctly and agy successfully processes the CRITIQUE_RUN protocol.
Validation
• Tested manually on Windows by executing a complex design prompt (@Creative-Director) through the Open Design UI with agentId: antigravity configured.
• Confirmed that the temporary file uses a PID-based naming strategy (od_agy_prompt_${process.pid}.md) to safely overwrite itself on each turn without
causing temp directory bloat over time.